Skip to content

[BUILD] Add rules_cc load statements #3697

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom
Open
wants to merge 2 commits into
base: main
Choose a base branch
from
Open

[BUILD] Add rules_cc load statements #3697

wants to merge 2 commits into from
+173 −14

Conversation

keith
Copy link
Contributor

@keith keith commented Oct 16, 2025

With the next major version of bazel this becomes required. This change
was automated with buildifier -r . --lint=fix

@keith
[BUILD] Add rules_cc load statements
aeb099a 
With the next major version of bazel this becomes required. This change
was automated with `buildifier -r . --lint=fix`
@keith keith requested a review from a team as a code owner October 16, 2025 00:56
@codecov Codecov
Copy link

codecov bot commented Oct 16, 2025 •
edited
Loading

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 89.95%. Comparing base (767ef30) to head (813dc8f).
⚠️ Report is 3 commits behind head on main.

Additional details and impacted files

Impacted file tree graph

@@           Coverage Diff           @@
##             main    #3697   +/-   ##
=======================================
  Coverage   89.95%   89.95%           
=======================================
  Files         225      225           
  Lines        7273     7273           
=======================================
  Hits         6542     6542           
  Misses        731      731
🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

lalitb
lalitb approved these changes Oct 16, 2025
View reviewed changes
Copy link
Member

@lalitb lalitb left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks. Not a bagel expert, but trusting the buildifier gods on this one :)
Seems the CI failure is not related to the PR, something related to vcpkg. cc @ThomsonTan

@lalitb
Copy link
Member

lalitb commented Oct 16, 2025 •
edited
Loading

Approved earlier, but noticed another CI failure (Cannot load '@@rules_cc//cc:cc_library.bzl': no such file).
Likely missing rules_cc declaration in WORKSPACE or MODULE.bazel — seems Bazel 7 now requires it explicitly.

make command: make -k -j $(nproc)
CMAKE_OPTIONS: -DCMAKE_BUILD_TYPE=Debug -DCMAKE_CXX_STANDARD=17 -DCMAKE_CXX_STANDARD_REQUIRED=ON -DCMAKE_CXX_EXTENSIONS=OFF
2025/10/16 00:59:13 Downloading https://releases.bazel.build/7.1.1/release/bazel-7.1.1-linux-x86_64...
Starting local Bazel server and connecting to it...
Computing main repo mapping: 
WARNING: Target pattern parsing failed.
ERROR: Skipping '//exporters/otlp:with_otlp_grpc_credential_preview': error loading package 'exporters/otlp': cannot load '@@rules_cc//cc:cc_library.bzl': no such file
ERROR: //exporters/otlp:with_otlp_grpc_credential_preview :: Error loading option //exporters/otlp:with_otlp_grpc_credential_preview: error loading package 'exporters/otlp': cannot load '@@rules_cc//cc:cc_library.bzl': no such file
Error: Process completed with exit code 2.

@keith
Copy link
Contributor Author

keith commented Oct 16, 2025

thanks, wdyt about dropping WORKSPACE support now? #3699

I can fix it here but it might be a reasonable time to drop anyways

marcalff
marcalff approved these changes Oct 17, 2025
View reviewed changes
Copy link
Member

@marcalff marcalff left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM, thanks for the fix.

@marcalff
Copy link
Member

marcalff commented Oct 17, 2025 •
edited
Loading

CI failing:

@marcalff
Copy link
Member

marcalff commented Oct 18, 2025 •
edited
Loading

Thanks @keith

Any chance you can take a look at opentelemetry-proto 1.8.0 in BCR ?

@marcalff marcalff added the pr:fix-merge-conflicts Please fix merge conflicts for this pr label Oct 18, 2025
@marcalff
Copy link
Member

3 files with changes got removed.

I was expecting to resolve this using the web editor, but it looks the github UI does not allow that, this is "too complex".

@keith Please take a look.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@lalitb lalitb lalitb approved these changes

@marcalff marcalff marcalff approved these changes

Assignees

No one assigned

Labels

pr:fix-merge-conflicts Please fix merge conflicts for this pr

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

3 participants

@keith @lalitb @marcalff